Add size to versions #484
Add size to versions #484
Conversation
…ptional parameter.
…s for existing gems
| @@ -28,6 +28,15 @@ def save | |||
| end | |||
| end | |||
|
|
|||
| def size_gem | |||
| if body | |||
| @size = body.size | |||
There was a problem hiding this comment.
You could probably memoize this
@size ||= body.size
There was a problem hiding this comment.
Not sure it's really necessary if we only use it once.
| @@ -4,6 +4,9 @@ | |||
| <% if version.platformed? %> | |||
| <span class="platform"><%= version.platform %></span> | |||
| <% end %> | |||
| <% if version.sized? %> | |||
| <span class="size">(<%= number_to_human_size(version.size) %>)</span> | |||
There was a problem hiding this comment.
Can we get rid of the if and make version.size return "NA" if there is no size? It would follow tell don't ask, and make the view more consistant on how it looks?
There was a problem hiding this comment.
I'm fine with this approach. N/A sounds fine to me!
|
@gnarg Are you still working on this or do you need someone else to finish it off? I don't want to sweep in if someone is actively working on this. |
|
Hi, sorry, yeah, I'll incorporate the suggestions here and resubmit, hopefully today. I was trying to get caught up after RubyConf, but I think I have some time now. The Hack-a-thon / BoF thing at RubyConf was a lot of fun and I'd like to get this landed if possible. Thanks. |
|
Cool, just let us know when it's good to go, we'll get it merged and coordinate the deployment with the rake task to backfill the version numbers. Thanks! |
|
Ok, I think I incorporated all the given suggestions. Should be ok to merge unless there are more ideas for improvements. |
|
|
or 🚢 it
| Version.find_in_batches do |versions| | ||
| versions.each do |version| | ||
| print "sizing #{version.full_name}: " | ||
| file = fog.directories.get($rubygems_config[:s3_bucket] + '/gems') \ |
There was a problem hiding this comment.
This... is going to take a really, really long time.
1.9.3p0 :001 > Version.indexed.count
(172.1ms) SELECT COUNT(*) FROM "versions" WHERE "versions"."indexed" = 't'
=> 253733
Maybe there's a better way we can do this with S3? Some kind of export of data? /cc @geemus
There was a problem hiding this comment.
Why can't we roll some ec2 instances to compute the sizes of all of the gems on s3 and create a file of sizes and then use that to import on the main gemcutter instances? This way, i/o to AWS is free for all of the EC2 operations and we can leave it running somewhere without worrying about site performance?
There was a problem hiding this comment.
We can absolutely spin up EC2 instances to update the sizes. That can be done async after this change is merged in.
|
Reviewed, this is not ready to merge yet. |
| @@ -46,7 +55,7 @@ def pull_spec | |||
|
|
|||
| def find | |||
| @rubygem = Rubygem.find_or_initialize_by_name(spec.name) | |||
| @version = @rubygem.find_or_initialize_version_from_spec(spec) | |||
| @version = @rubygem.find_or_initialize_version_from_spec_and_size(spec, size) | |||
There was a problem hiding this comment.
Why would we use the size to look up the version? Why would we create the same version with different sizes?
There was a problem hiding this comment.
Oh, I see. The method is named badly, it doesn't actually use the size to search. As @qrush says below, nuke this method.
|
I cleaned up some of the objectionable code. @cmeiklejohn I hope you don't mind too much that I'm basically rewriting your code here. |
|
Any more feedback on this? It's worth noting that the travis failure was a transient github outtage: |
|
@qrush What should be done on this to make it ready? |
|
Bueller? |
|
Merging and deploying tomorrow (7/12/2013) |
Up to date version of cmeiklejohn's pull request #310